-
-
Notifications
You must be signed in to change notification settings - Fork 314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove jtreg duplicated crypto call #5134
Remove jtreg duplicated crypto call #5134
Conversation
which is doing moreover the same
be0b4cd
to
8ec9800
Compare
From the grinder console output jtreg.jar are actually downloaded twice. One is downloaded by build.xml the other one is downloaded by run.sh. Can run.sh update to only download if no jtreg.jar exist? |
I had removed the build's download of jtreg. https://ci.adoptium.net/view/Test_grinder/job/Grinder/9150/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, by removing this call and depending on the test material being added, in this case 'cryptotest', to fetch 'some unknown/untracked version of jtreg' we lose the ability to reproduce the exact same test material. We also set a bad precedent for other tests coming in the future that it is okay to pull in extra untracked dependencies and glut the test machines of anyone using AQAvit.
Sure! Cuple of question on this topic.:
Where I absolutely agree with what you wrote, few things I would like to understand:
|
The jtreg that gets pulled and used certainly depends on the JDK version, and AQAvit will pull the 'right one' based on JDK_VERSION. If we are assessing which foreign repos to include into AQAvit, one criteria would be that they integrate well into the system we have in place. My bad for not seeing that you were pulling your own copy of jtreg, that was a failure to review closely during my reviews. When dovetailing into AQAvit, the point is to centralize functionality. If contributors still want to have the option to download a bespoke version of jtreg, then that can be controlled through an option passed to the test suite. Of the ~10 companies using the tests being added into AQAvit, the expectation is to know what is being downloaded onto their test machines and that the AQAvit project does some vetting/verification of those dependencies. When does this openjdk/jtreg@8c105f2 land into an official tagged/versioned release of jtreg? If it is super-important, then it will be important that its into an official version, that we can then plan to pick up. |
Which will make openjdk/jtreg@8c105f2 missing again, but that - a tleast for jdk bigger then 8, should be eg from https://ci.adoptium.net/job/dependency_pipeline/lastSuccessfulBuild/artifact/jtreg/jtregtip.tar.gz
cool! There is also https://ci.adoptium.net/view/Dependencies/job/dependency_pipeline/lastSuccessfulBuild/artifact/jtreg/jtregtip.tar.gz ; any way to pull this?
It is only on those ssl/crypto runner PRs. At it is impossibel to see without properly reading thre run.sh. Kudos for @sophia-guo for noticing that!
I agree for sure. Can it be subject of discussion for some very special cases and deps? but this is absolutely not that case.
No idea. Jtreg release is random AFAICT. Maybe around jdk24? And wq can be sure it will not reach the |
Yes, let's plan to add jtreg tip to the list of ones that can be used with AQAvit testing, in that way it satisfies both our needs. |
TYVM! Can this go in in meantime? (+5136 and maybe 5129 :) |
There is now a failure in OpenJ9 builds which I assume is caused by this change.
|
Failed differently on zlinux.
|
Issue introduced by this change rh-openjdk/CryptoTest#63 using |
Created #5151 with the plan to pin to specific versions of these test materials so we are not at the mercy of random changes. |
Excluded the test on OpenJ9 amac, zlinux via #5153 |
* Removed jtreg call by the upstream wrapper which is doing moreover the same * moved out of the tmpresults * Removed build's download of jtreg as suite pulls its own * Returned back to aqavit's jtregs withotu downloading custom one Which will make openjdk/jtreg@8c105f2 missing again, but that - a tleast for jdk bigger then 8, should be eg from https://ci.adoptium.net/job/dependency_pipeline/lastSuccessfulBuild/artifact/jtreg/jtregtip.tar.gz
- add CryptoTests_jtreg back (see adoptium#5134) - CryptoTests_jtreg directly triggers the tests via jtreg and it runs against all vendors except redhat - CryptoTests only runs for vendor=redhat - add retry for git clone to mitigate network issue - add check CryptoTest repo sha Signed-off-by: Lan Xia <[email protected]>
- add CryptoTests_jtreg back (base on the original change adoptium#5134) - CryptoTests_jtreg directly triggers the tests via jtreg and it runs against all vendors except redhat - CryptoTests only runs for vendor=redhat - add retry for git clone to mitigate network issue - add check CryptoTest repo sha Signed-off-by: Lan Xia <[email protected]>
- add CryptoTests_jtreg back (based on the original change adoptium#5134) - CryptoTests_jtreg directly triggers the tests via jtreg and it runs against all vendors except redhat - CryptoTests only runs for vendor=redhat - re-enable CryptoTests_jtreg on aarch64_mac|x86-64_mac|s390x_linux - add retry for git clone to mitigate network issue - add check CryptoTest repo sha (sha will be captured in TAP file) Signed-off-by: Lan Xia <[email protected]>
Based on the experience (hopefully correct!) simplifying cryptotest runner
https://ci.adoptium.net/view/Test_grinder/job/Grinder/9093